Implement extends #536
Implement extends #536
Conversation
jalil-salame
force-pushed
the
implement-extends
branch
from
5d51e98 to
8a21213
Compare
The new syntax files cause new test to fail (and C# to pass). This should be looked over more carefully and fixed in the future.
This adds support for the `extends` keyword. The implementation is very hacky right now: - We now require an optional `base_syntax` when parsing a syntax, if the `base_syntax` is not present and we find an `extends` keyword at the top level, we error out reporting which syntax file we expected. - SyntaxSet collects this error and resolves the syntaxes in a loop until no more syntaxes can be resolved. - We don't handle multiple-inheritance (`extends` might be a list). - We don't re-evaluate the `contexts` as defined in the spec (you can override variables and have that affect the context). - We only handle `extends` for syntaxes added through `SyntaxSet::load_from_folder`.
jalil-salame
force-pushed
the
implement-extends
branch
from
8a21213 to
a712440
Compare
|
I forgot to say this at the time, but thanks very much for your hard work on this! It is very much appreciated! 🚀
The match patterns are currently evaluated when the syntax definition is parsed. In |
|
I notice there is an idea to delay expansion of variables - #541 (comment) - perhaps we should also consider that as a nicer solution to the problem of allowing extended syntaxes to use the overridden variables? |
2 participants
This adds support for the
extendskeyword which lets the tests pass on Build 4075 as requested in #323. The implementation is very hacky right now:base_syntaxwhen parsing a syntax, if thebase_syntaxis not present and we find anextendskeyword at the top level, we error out reporting which syntax file we expected.SyntaxSetBuilderhandles this error and resolves the syntaxes in a loop until no more syntaxes can be resolved.extendsmight be a list).contextsas defined in the spec (you can override variables and have that affect the context).extendsfor syntaxes added throughSyntaxSetBuilder::load_from_folder.I am opening this PR on top of #535 to get the fixes to the tests.
I don't know if this is a correct implementation of the
extendskeyword, specifically, I don't know when contexts are evaluated; the contexts are properly merged, but the variables can be overwritten and thus the context needs to be re-evaluated, I don't know if I need to do something about it, or id the context evaluation is done after theSyntaxSetis built.I am not happy with the implementation either, but I don't know how to improve it further. I will happily improve it if you have any ideas.
Ideally,
SyntaxSetBuilder::buildwould return an error that indicates the syntax was unable to extend another syntax.